Drop arm as a default CPU ABI
#1894
Conversation
|
Hold up, missed some tests. Will fix. Edit: Fixed. |
LegNeato
force-pushed
the
remove_arm_default
branch
from
63f52d7
to
17f0b70
Compare
|
@LegNeato has updated the pull request. |
LegNeato
force-pushed
the
remove_arm_default
branch
from
17f0b70
to
e08ba85
Compare
|
@LegNeato has updated the pull request. |
LegNeato
force-pushed
the
remove_arm_default
branch
from
e08ba85
to
a53fe41
Compare
|
@LegNeato has updated the pull request. |
LegNeato
force-pushed
the
remove_arm_default
branch
from
a53fe41
to
48785a5
Compare
|
@LegNeato has updated the pull request. |
With the latest version of the NDK, `arm` is not supported. Fixes facebook#1890.
LegNeato
force-pushed
the
remove_arm_default
branch
from
48785a5
to
b6c985a
Compare
|
@LegNeato has updated the pull request. |
There was a problem hiding this comment.
Let's make the list of ABI depend on the NDK version. This would prevent an issue with existing clients that do not use v17.
Also, I think we should keep all the tests as they were before, but add assume checks to verify the version of NDK and disable tests that do not work with v17. We should also provide modified versions of those tests for v17.
|
@styurin I am not sure I agree about not modifying the tests...if you look at them they are not testing anything I of course will add tests to check the new behavior of switching defaults based on NDK versions. |
|
Ping @styurin |
|
Sounds good. |
There was a problem hiding this comment.
@styurin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@styurin any update on the import? |
|
I'm working on it, there is another issue with NDK 16 which should be resolved before allowing NDK 17. |
|
I believe this is actually still on me...I was waiting for my other PR to land before I rebased and fixed comments |
|
Implemented in 4ef8e96. |
|
Sweet, thanks! |
With the latest version of the NDK,
armis not supported.Fixes #1890.
This could possibly be a breaking change for people who have not set CPU ABI and are not using the latest NDK---before their builds would have
arm, after updating Buck they will no longer and might wonder what happened. Open to strategies to dealing with that, though they can always just addarmtocpu_abisin their config to fix any bustage. While buck could detect this case, it seems like a bit too much magic to be worth it.Note that this is somewhat related to #1889 but not exactly.